feat: alert component #3680
feat: alert component #3680
Conversation
🦋 Changeset detectedLatest commit: eebab5d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@abhinav700 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a new Alert component to the project, incorporating dependencies from the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
wingkwong
changed the title
…per, adding isCloseable prop
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/docs/content/docs/components/alert.mdx (1)
95-111: Enhance API table readability.The API section provides comprehensive information about the Alert component's props and events. To improve readability, consider the following suggestion:
- Use a more compact format for the
colorprop options. For example:| Attribute | Type | Description | Default | | --------- | ---- | ----------- | ------- | | color | `"default"` \| `"primary"` \| `"secondary"` \| `"success"` \| `"warning"` \| `"danger"` | The alert color theme. | `"default"` |This format is more concise and easier to read, especially for props with multiple options.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- apps/docs/content/docs/components/alert.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/alert.mdx
[grammar] ~71-~71: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...closeButton**: ThecloseButton, it is at the top-right corner of alert. - description: The descri...(ON_IN_THE_CORNER_2)
🔇 Additional comments (6)
apps/docs/content/docs/components/alert.mdx (6)
1-13: LGTM: Header section is well-structured and informative.The header section provides clear metadata and a concise description of the Alert component. The inclusion of the ComponentLinks is a good practice for navigation.
18-29: LGTM: Comprehensive installation instructions provided.The installation section covers multiple package managers, making it easy for users with different setups to install the component. The inclusion of a global install warning is a good practice for preventing potential issues.
31-39: LGTM: Clear import instructions provided.The import section effectively uses the ImportTabs component to demonstrate both main package and individual component import options, catering to different user preferences and project setups.
41-62: Improve clarity and fix minor grammar issues in the usage section.The usage section is informative, but there are a few areas that could be improved:
Lines 42-43: Consider using a
<Blockquote>component for better visual hierarchy:<Blockquote> It accepts `title` and `description` as props for the alert message. `title` is an optional property. </Blockquote>Line 58: Improve clarity by changing to:
"IfisClosableis true, a close button appears on the alert, which can be used to close it."Line 60: Add "the" before "isClosable property":
"If theonCloseis passed, the close button is visible regardless of theisClosableproperty."Additionally, consider adding brief explanations or examples for the "Radius" and "Colors" subsections to enhance user understanding of these customization options.
67-74: Fix minor grammar issue in the slots section.The slots section provides valuable information for customizing the Alert component. However, there's a minor grammar issue that should be addressed:
Line 71: Change "it is at the top-right corner of alert" to "it is in the top-right corner of the alert".
Line 74: For consistency, consider changing "close icon that is wrapped" to "The close icon that is wrapped".
These small changes will improve the overall clarity and consistency of the documentation.
🧰 Tools
🪛 LanguageTool
[grammar] ~71-~71: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...closeButton**: ThecloseButton, it is at the top-right corner of alert. - description: The descri...(ON_IN_THE_CORNER_2)
76-91: LGTM: Comprehensive customization options provided.The custom styles and implementation section offers valuable information for advanced users:
- Clear instructions on using Tailwind CSS classes for customization.
- Introduction of the
useAlerthook for more complex implementations.- Appropriate use of CodeDemo components to illustrate the concepts.
This section effectively caters to users who need more control over the Alert component's appearance and behavior.
|
@abhinav700 i think the reason is you have the fixed width so it won't shrink when the window is resizing. You may try if you put two alert on the same row and resize the container. Adding breakpoint doesn't solve the full problem since it may not reach the breakpoint. |
I think my original code would work if I replace responsive.mp4 |
|
Let's discuss in discord. |
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (7)
apps/docs/content/components/alert/colors.ts (1)
1-27: Overall assessment: Improvements needed to fully meet PR objectivesThe implementation successfully introduces the Alert component demo with various color options. However, there are several areas for improvement:
- Component structure: Consider refactoring to a separate
.tsxfile for better maintainability and type safety.- Dismissibility: Implement the ability for users to dismiss alerts, as mentioned in the PR objectives.
- Content management: Use constants or a configuration object for alert content to facilitate updates and potential internationalization.
- File extension consistency: Ensure the file extension in the
reactobject matches the actual file type.To fully meet the PR objectives and improve the overall quality of the implementation, please address the suggestions provided in the previous comments. This will enhance the component's functionality, maintainability, and alignment with the project's goals.
If you need any assistance in implementing these changes or would like to discuss alternative approaches, please don't hesitate to ask. We can also create GitHub issues to track these improvements if needed.
packages/components/alert/stories/alert.stories.tsx (2)
1-36: LGTM! Consider enhancing the decorator for better responsiveness.The imports and Storybook configuration look good. The decorator effectively centers the stories, which is great for presentation.
Consider updating the decorator to be more responsive:
decorators: [ (Story) => ( - <div className="flex items-center justify-center w-screen h-screen"> + <div className="flex items-center justify-center min-h-screen p-4"> <Story /> </div> ), ],This change ensures proper padding on smaller screens and uses
min-heightinstead of a fixed height for better responsiveness.
101-122: LGTM! Consider adding a comment about potential style conflicts.The CustomWithClassNames story effectively demonstrates how to apply custom styles to the Alert component using Tailwind CSS classes.
Consider adding a comment to warn about potential style conflicts:
export const CustomWithClassNames = { render: Template, args: { ...defaultProps, classNames: { // Note: These custom styles may override or conflict with the component's built-in styles. // Ensure to test thoroughly when applying custom styles. base: [ // ... (existing classes) ], // ... (other custom classes) }, }, };This comment will remind developers to be cautious when applying custom styles and to test thoroughly for any unintended side effects.
apps/docs/content/docs/components/alert.mdx (2)
41-62: Improve grammar and structure in the usage section.While the content is informative, there are a few areas for improvement:
- Line 43: Change "a optional" to "an optional" to fix the grammar.
- Line 58: Consider rephrasing to "If
isClosableis true, a close button appears on the alert, which can be used to dismiss it."- Lines 60-61: Use a
<Blockquote>component for the note about theonCloseprop to improve visual hierarchy.Consider applying these changes:
-- `title` is a optional property. -- `title` is an optional property. -If `isClosable` is true, a close button appears on the alert, which can be used to close it. -If `isClosable` is true, a close button appears on the alert, which can be used to dismiss it. -> **Note**: If the `onClose` is passed, the close button is visible regardless of the `isClosable` property. +<Blockquote> + If the `onClose` prop is passed, the close button is visible regardless of the `isClosable` property. +</Blockquote>
67-92: Improve grammar in the slots section and LGTM for content.The slots section provides valuable information about customizing the Alert component. However, there are a few grammar issues to address:
- Line 71: Change "it is at the top-right corner of alert" to "it is in the top-right corner of the alert".
- Line 74: Change "close icon that is wrapped" to "the close icon that is wrapped".
- Line 75: Change "icon that appears at the top-left corner" to "icon that appears in the top-left corner".
Consider applying these changes:
-- **closeButton**: The `closeButton`, it is at the top-right corner of alert. -- **closeButton**: The `closeButton`, it is in the top-right corner of the alert. -- **closeIcon**: close icon that is wrapped inside the `closeButton`. -- **closeIcon**: The close icon that is wrapped inside the `closeButton`. -- **alertIcon**: icon that appears at the top-left corner. -- **alertIcon**: Icon that appears in the top-left corner.The examples for custom styles and custom implementation are well-presented and provide good guidance for developers looking to customize the Alert component.
🧰 Tools
🪛 LanguageTool
[grammar] ~71-~71: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...closeButton**: ThecloseButton, it is at the top-right corner of alert. - description: The descri...(ON_IN_THE_CORNER_2)
[grammar] ~75-~75: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...on`. - alertIcon: icon that appears at the top-left corner. ### Custom Styles You can customize ...(ON_IN_THE_CORNER_2)
packages/components/alert/src/alert.tsx (1)
84-84: Simplify the return statement by removing the unnecessary fragmentSince
baseWrapperis a single element, you can return it directly without wrapping it in a fragment.Apply this diff:
- return <>{baseWrapper}</>; + return baseWrapper;packages/components/alert/src/use-alert.ts (1)
17-17: Ensure consistent capitalization in commentsSome comments begin with lowercase letters (e.g., "title of the alert message", "whether the alert can be closed by user", "function which is called when close button is clicked"). For consistency and readability, consider starting all comments with a capital letter.
Also applies to: 27-27, 32-32
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- apps/docs/content/components/alert/colors.ts (1 hunks)
- apps/docs/content/components/alert/custom-impl.ts (1 hunks)
- apps/docs/content/components/alert/radius.ts (1 hunks)
- apps/docs/content/docs/components/alert.mdx (1 hunks)
- packages/components/alert/src/alert.tsx (1 hunks)
- packages/components/alert/src/use-alert.ts (1 hunks)
- packages/components/alert/stories/alert.stories.tsx (1 hunks)
- packages/core/theme/src/components/alert.ts (1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/alert.mdx
[grammar] ~71-~71: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...closeButton**: ThecloseButton, it is at the top-right corner of alert. - description: The descri...(ON_IN_THE_CORNER_2)
[grammar] ~75-~75: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...on`. - alertIcon: icon that appears at the top-left corner. ### Custom Styles You can customize ...(ON_IN_THE_CORNER_2)
🔇 Additional comments (9)
apps/docs/content/components/alert/colors.ts (1)
25-27: LGTM: Export structure is appropriateThe export structure using object spread is correct and allows for easy expansion if more components need to be added in the future. This approach is flexible and maintainable.
apps/docs/content/docs/components/alert.mdx (5)
1-13: LGTM: Title and description are clear and concise.The title, description, and component links are well-structured and provide a good introduction to the Alert component.
18-29: LGTM: Comprehensive installation instructions provided.The installation section covers multiple package managers and includes a global install warning, which is appropriate for a component library.
31-39: LGTM: Clear import instructions provided.The import section clearly shows how to import the Alert component from both the main package and the individual component package.
96-107: LGTM: Comprehensive API documentation.The API section provides a clear and well-structured table of props for the Alert component, including descriptions, types, and default values. This information is valuable for developers implementing the component.
108-112: LGTM: Clear events documentation.The Events section succinctly describes the
onCloseevent for the Alert component, providing developers with the necessary information for handling alert dismissals.packages/components/alert/src/alert.tsx (1)
17-32: Verify all necessary props are destructured fromuseAlertEnsure that all required properties returned by
useAlertare destructured. Missing props could lead to unexpected behavior.Run the following script to list all exported properties from
useAlert:Compare the list of properties with the destructured variables to ensure none are missing.
✅ Verification successful
All necessary props are correctly destructured from
useAlert.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: List all exports from 'useAlert' to ensure all necessary props are destructured. # Test: List all returned properties from 'useAlert'. Expect: No missing properties in destructuring. rg 'return {(.*)}' packages/components/alert/src/use-alert.ts | sed -E 's/return {(.*)}/\1/' | tr ',' '\n' | sortLength of output: 273
Script:
#!/bin/bash # Description: Extract all properties returned by 'useAlert' using ast-grep. ast-grep --lang typescript --pattern 'return { $$$ }' packages/components/alert/src/use-alert.tsLength of output: 2774
packages/core/theme/src/components/alert.ts (2)
36-78:⚠️ Potential issueVerify the custom color utilities used in color variants.
The color variants utilize classes like
bg-default-100,text-default-600,text-primary,text-primary-200, etc. These are not part of Tailwind CSS's default color palette.Please ensure that these color utilities are defined in your Tailwind CSS configuration under the
theme.colorssection. If they are not defined, the styles may not render as expected. Alternatively, consider using Tailwind's default colors likebg-blue-100,text-gray-600, etc.Run the following script to check for custom color definitions:
#!/bin/bash # Description: Verify custom colors in Tailwind CSS configuration. # Test: Search for custom colors such as 'default', 'primary', 'secondary', 'success', 'warning', 'danger'. # Expected: Definitions of the custom colors in the Tailwind config. # Find Tailwind config files and search for the custom color definitions. fd --type file --regex 'tailwind\.config\.(js|ts)' --exec cat {} \; | \ grep -E 'colors.*(default|primary|secondary|success|warning|danger)'
25-34:⚠️ Potential issueVerify the usage of potential non-standard Tailwind CSS classes in slots.
The slots are using classes like
text-medium,text-small,min-h-17, andms-5. Some of these may not exist in Tailwind CSS's default configuration:
text-mediumandtext-smallare not standard font size utilities.min-h-17may not be a default spacing utility.ms-5is the margin-start utility for RTL support; ensure it's configured correctly.Please verify that these classes are defined in your Tailwind CSS configuration or consider using standard classes like
text-base,text-sm,min-h-16, etc., for better compatibility and maintainability.Run the following script to check if these custom classes are defined:
|
Okay, will look into that |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
packages/components/alert/src/index.ts (2)
3-4: LGTM: AlertProps type export with a minor suggestionThe export of the AlertProps type is correctly implemented, allowing users to access the component's prop types. This is crucial for type checking and autocompletion in TypeScript projects.
For consistency with the other exports, consider moving the comment to its own line:
-// export types -export type {AlertProps} from "./alert"; +// export types +export type {AlertProps} from "./alert";
1-10: Overall structure looks great, with a minor suggestionThe file structure is clean, efficient, and follows common patterns for React component libraries. It effectively serves as an entry point for the Alert component module, exporting all necessary components, types, and hooks.
For improved readability and maintainability, consider grouping related imports at the top of the file:
import Alert from "./alert"; +import type {AlertProps} from "./alert"; +import {useAlert} from "./use-alert"; // export types -export type {AlertProps} from "./alert"; +export type {AlertProps}; // export hooks -export {useAlert} from "./use-alert"; +export {useAlert}; // export component export {Alert};This change would centralize all imports at the top of the file, making it easier to manage dependencies in the future.
apps/docs/content/docs/components/alert.mdx (3)
31-63: Improve clarity and fix grammar in the usage sectionPlease address the following points to enhance the documentation:
Lines 42-43: Wrap the information in a
<Blockquote>component for better visual hierarchy:<Blockquote> It accepts `title` and `description` as props for the alert message. `title` is an optional property. </Blockquote>Line 58: Improve the sentence structure:
- If `isClosable` is true, a close button appears on the alert, which can be used to close it. + If `isClosable` is true, a close button appears on the alert that can be used to close it.Line 60: Add "the" before "isClosable property":
- > **Note**: If the `onClose` is passed, the close button is visible regardless of isClosable property. + > **Note**: If the `onClose` is passed, the close button is visible regardless of the `isClosable` property.Consider adding brief explanations or examples for each customization option (radius, colors) to enhance user understanding.
64-105: Improve grammar and clarity in the slots sectionPlease make the following improvements to enhance the documentation:
Line 72: Change the preposition and add "the" before "alert":
- The `closeButton`, it is at the top-right corner of alert. + The `closeButton`, it is in the top-right corner of the alert.Line 75: Add "the" before "close icon":
- close icon that is wrapped inside the `closeButton`. + The close icon that is wrapped inside the `closeButton`.Line 82: Change the preposition:
- icon that appears at the top-left corner. + Icon that appears in the top-left corner.Consider adding more details or examples for each slot to provide better guidance on how to customize them.
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...oseButton**: ThecloseButton, it is at the top-right corner of alert. - description: The desc...(ON_IN_THE_CORNER_2)
[grammar] ~82-~82: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...`. - alertIcon: icon that appears at the top-left corner. ### Custom Alert St...(ON_IN_THE_CORNER_2)
109-125: Minor inconsistency in API tableThe API table is well-structured and informative. However, there's a small inconsistency that should be addressed:
- Line 119: The default value for
isClosableis shown asfalse, but it's formatted differently from other default values in the table. Update it for consistency:- | isClosable | `boolean` | Whether the close button should be displayed. | `false` | + | isClosable | `boolean` | Whether the close button should be displayed. | `false` |Consider adding a brief description of the
Alertcomponent at the beginning of the API section to provide context for the props and events.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
- apps/docs/content/docs/components/alert.mdx (1 hunks)
- packages/components/alert/src/index.ts (1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/alert.mdx
[grammar] ~74-~74: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...oseButton**: ThecloseButton, it is at the top-right corner of alert. - description: The desc...(ON_IN_THE_CORNER_2)
[grammar] ~82-~82: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...`. - alertIcon: icon that appears at the top-left corner. ### Custom Alert St...(ON_IN_THE_CORNER_2)
🔇 Additional comments (5)
packages/components/alert/src/index.ts (2)
1-1: LGTM: Alert component import and exportThe import and export of the Alert component are correctly implemented. This structure allows for clean and intuitive importing of the component by users of the library.
Also applies to: 10-10
6-7: LGTM: useAlert hook exportThe export of the useAlert hook is correctly implemented. This allows users to access the hook functionality separately if needed, providing flexibility in how the Alert component can be used and customized.
apps/docs/content/docs/components/alert.mdx (3)
1-17: LGTM: Well-structured header and introductionThe header and introduction sections are well-organized, providing clear information about the Alert component. The use of ComponentLinks and CarbonAd components enhances the documentation's functionality.
18-30: LGTM: Comprehensive installation instructionsThe installation section provides clear instructions for various package managers, making it easy for users to integrate the Alert component into their projects.
1-125: Overall: Well-structured and comprehensive documentationThe Alert component documentation is well-organized and provides valuable information for users. The suggested improvements mainly focus on enhancing clarity, fixing minor grammar issues, and ensuring consistency throughout the document. Once these changes are implemented, the documentation will be even more effective in guiding users on how to use and customize the Alert component.
Key areas of improvement:
- Enhancing the usage section with better visual hierarchy and clearer explanations.
- Refining grammar and preposition usage in the slots section.
- Adding more detailed examples for customization options.
- Ensuring consistency in the API table.
Great job on creating this comprehensive documentation for the Alert component!
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-right corner”?
Context: ...oseButton**: ThecloseButton, it is at the top-right corner of alert. - description: The desc...(ON_IN_THE_CORNER_2)
[grammar] ~82-~82: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...`. - alertIcon: icon that appears at the top-left corner. ### Custom Alert St...(ON_IN_THE_CORNER_2)
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
packages/core/theme/src/components/alert.ts (1)
4-23: Enhance JSDoc example with variant usage.The current example demonstrates basic structure but could be more helpful by showing:
- How to use color and radius variants
- How the slots can be composed with variants
Example enhancement:
/** * @example * ```js * const {base, mainWrapper, title, description, closeButton, alertIcon} = alert({ * color: "primary", * radius: "lg" * }); * * <div className={base()}> * <div className={alertIcon()}> * <InfoIcon /> * </div> * <div className={mainWrapper()}> * <div className={title()}>Title</div> * <div className={description()}>Description</div> * </div> * <button className={closeButton()}> * <CloseIcon /> * </button> * </div> * ``` */apps/docs/content/docs/components/alert.mdx (3)
42-43: Enhance readability with proper component usage.The usage information should be wrapped in a
<Blockquote>component for better visual hierarchy and consistency with other documentation pages.Apply this change:
- - It accepts `title` and `description` as props for the alert message. - - `title` is an optional property. +<Blockquote> + - It accepts `title` and `description` as props for the alert message. + - `title` is an optional property. +</Blockquote>
82-82: Fix preposition in alertIcon description.Change the preposition for better grammatical accuracy.
Apply this change:
- icon that appears at the top-left corner. + icon that appears in the top-left corner.🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...`. - alertIcon: icon that appears at the top-left corner. ### Custom Alert St...(ON_IN_THE_CORNER_2)
113-120: Consider documenting additional common props.The API documentation could be enhanced by including common React props that the component likely supports:
className: For applying custom CSS classesstyle: For inline styles- HTML attributes that are forwarded to the root element
This would help developers understand all available customization options.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- apps/docs/content/components/alert/custom-impl.ts (1 hunks)
- apps/docs/content/docs/components/alert.mdx (1 hunks)
- packages/components/alert/src/alert.tsx (1 hunks)
- packages/components/alert/src/use-alert.ts (1 hunks)
- packages/core/theme/src/components/alert.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/alert/src/alert.tsx
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/alert.mdx
[grammar] ~82-~82: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the top-left corner”?
Context: ...`. - alertIcon: icon that appears at the top-left corner. ### Custom Alert St...(ON_IN_THE_CORNER_2)
🔇 Additional comments (8)
packages/core/theme/src/components/alert.ts (2)
24-34: LGTM! Well-organized slot structure.The slot structure is well-organized with clear separation of concerns between base layout, main content wrapper, title, description, and interactive elements.
98-107: LGTM! Well-structured exports and sensible defaults.The default variants and TypeScript types are well-defined:
- Default color variant is "default" which provides neutral styling
- Default radius is "md" which aligns with common UI patterns
- Exported types ensure type safety when using the component
packages/components/alert/src/use-alert.ts (3)
4-4: Use package entry point for imports.The import from
@nextui-org/theme/src/components/alertis accessing an internal path. Consider importing from the package's main entry point@nextui-org/themefor better maintainability.-import {AlertVariantProps} from "@nextui-org/theme/src/components/alert"; +import type {AlertVariantProps} from "@nextui-org/theme";
10-53: Props interface is well-structured and documented.The interface provides comprehensive JSDoc comments and follows the agreed-upon structure with optional title and mandatory description.
57-124: Hook implementation looks solid.The implementation is well-structured with proper memoization of slots and prop getters. The close functionality is implemented correctly, and the returned object provides all necessary props and methods.
apps/docs/content/components/alert/custom-impl.ts (3)
102-125:⚠️ Potential issueFix missing dependencies in useMemo hook
The
useMemohook forbaseWrapperis missinggetCloseIconPropsin its dependency array, which could lead to stale values.}, [ mainWrapper, isClosable, getCloseButtonProps, isVisible, domRef, getBaseProps, handleClose, color, onClose, getAlertIconProps, + getCloseIconProps, ]);Likely invalid or redundant comment.
84-86:⚠️ Potential issueRemove hardcoded title and description values
The component contains hardcoded values which limit its reusability. These should be passed as props from the parent component.
- // this is just for the example, the props bellow should be passed by the parent component - title: "Email Sent!!", - description: "You will get a reply soon",Likely invalid or redundant comment.
134-142: 🛠️ Refactor suggestionSimplify the export structure
The current export structure using string literals and file paths as keys is unconventional and could lead to maintenance issues. Consider using direct exports.
-const react = { - "/App.jsx": App, - "/InfoCircleIcon": InfoCircleIcon, - "/CloseIcon": CloseIcon, -}; - -export default { - ...react, -}; +export { InfoCircleIcon } from './InfoCircleIcon'; +export { CloseIcon } from './CloseIcon'; +export { MyAlert } from './MyAlert';Likely invalid or redundant comment.
|
please click re-request review once it's ready |
|
Hey @abhinav700 thanks for contributing and creating this component, please check my changes #3982 |
📝 Description
Summary by CodeRabbit
New Features
Documentation
Tests